Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[4주차] 변지혜 미션 제출합니다. #20

Open
wants to merge 26 commits into
base: master
Choose a base branch
from

Conversation

mod-siw
Copy link

@mod-siw mod-siw commented Nov 3, 2023

배포 링크

✉️📬✉️

결과 화면

친구 목록 채팅 목록 채팅방 프로필

KEY QUESTIONS

1. 디자이너로부터 받은 QA 목록

노션 링크
피그마 링크
QA 커밋 링크

2. SPA, Routing

Routing이란 URL 경로에 따라 그에 맞는 UI를 보여주는 것입니다. SPA는 웹의 동작방식에 대한 페턴 중 하나입니다. 하나의 페이지로 구성되어 있으며 초기에 모든 리소스를 다운받고 변경 사항이 있을 때만 기존 페이지를 수정합니다. SPA의 장점으로는 페이지 요청마다 리로딩으로 인한 깜빡거림이 없고, 데이터 캐싱 방식 덕분에 성능면에서 유리합니다.

채팅방 페이지, 프로필 페이지에 각각 개별 아이디를 주어 아이디별로 다른 데이터가 들어갈 수 있도록 작성했습니다. 이 과정에서 useParams가 굉장히 유용했습니다. 그 외에도 useLocation 등 react-router-dom을 활용해 페이지 간 이동 뿐만 아니라 데이터 상태 관리에 있어서도 다양한 도움을 받을 수 있어서 좋았습니다! 협업 프로젝트 시에 페이지 이동 방식에도 Link, useNavigate 등 조금 나뉘는 경우가 있었는데, 협업 시작 전에 이런 부분을 통일하고 시작하면 좋을 것 같습니다.
저는 처음 쓴 방식이 useNavigate라 쭉 이 방식을 쓰고 있는데, 다른 방식에도 장단점이 있더라구요. 다른 방식이 더 알맞는 상황이 온다면 다양하게 도전해보고 싶다고 생각했습니다!

3. 상태 관리

이번에 recoil을 처음 써보았는데, 주로 정적인 data(사용자 목록, 채팅 목록)를 가져와서 쓰는 것에 많이 사용했습니다. 상태 변화 및 업데이트에는 거의 사용하지 않았던 것이 개인적으로 아쉬웠습니다... recoil의 다양한 내용을 모두 적용해본 것은 아니지만, atom 뿐만 아니라 atomfamily, selector, selctorfamily 등 다양한 방식을 사용해보고 싶습니다. 그 외에도 커스텀 훅을 만들어서 반복되는 코드를 줄여보면 좋겠다고 생각했습니다.

과제에 첨부되어 있는 자료와 함께 찾아보았을 때, 상태 관리의 중요성을 다시금 느꼈습니다. 처음에는 막연하게 복잡한 방식 혹은 잘 접해보지 못한 방식일수록 좋은 방식일까 생각했었지만, 구현 기능에 따라 각각의 상태 관리 방법을 적재적소에 잘 사용하는 것이 중요하겠구나 깨닫는 계기가 되었습니다!

후기

타입스크립트, 리코일처럼 새로운 방식을 이용해 구현해볼 수 있는 과제였는데, 많이 헤매기도 헤맸지만 덕분에 좀 더 다양한 코드 작성 방식을 떠올려볼 수 있었던 과제였습니다. 스스로의 부족한 점도 많이 발견하는 동시에 앞으로 배워나가야 할 것들이 한참이라는 걸 다시금 깨달을 수 있었습니다...ㅎㅎ 혼자 하는 과제가 아니었다보니 디자이너 분을 실망시키고 싶지 않다는 생각이 좋은 자극제가 되어준 것 같아요. 추가하고 싶은 기능도 괜히 더 많아지고, 디자인이랑 다른 자잘한 부분들이 유난히 더 크게 보이고 그랬습니다. 마음을 따라주지 않은 부분들이 그만큼 더 아쉽기도 합니다🥲

새로운 것들을 익히느라 기존에 신경 쓰려 했던 반응형, 코드 효율성 등을 이전만큼 못 챙긴 것이 아쉽습니다. 시간 더 들여서 추가해보고 싶은 기능, 디테일 수정하고 싶은 부분 등이 계속 보여서 좀 더 수정해보도록 하겠습니다...

타입스크립트의 경우 초반에는 컴포넌트별로 interface를 작성했는데, 중복되는 구조가 많아 따로 폴더를 빼서 가져와서 사용하니 보다 편리했습니다. 초반에 만든 컴포넌트에는 아직 개별적인 interface가 있어서 추후 정리하고 싶습니다!

상태별 리로딩에 따라 어떤 방식을 쓰는 것이 효율적일지 고민하며 작성해보려 했는데, 막판에는 구현에 급급하여 코드가 조금 더러워진 것 같기도 합니다. 가독성 좋은 코드, 이해하기 쉬운 네이밍을 항상 꿈꾸지만 마음처럼 잘 되지는 않는 것 같습니다.

그리고 채팅 목록 검색의 경우, 사용자 목록 데이터와 채팅 목록 데이터를 모두 사용해서 필터링을 하느라 코드 작성하면서 이렇게 돌아돌아 필터링을 하는 게 맞나..! 싶기도 했어요. 리뷰해주실 때 다른 좋은 필터링 방법 있으면 알려주시면 감사하겠습니다😅

Copy link

@flowerseok flowerseok left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

시험기간이였는데 너무 고생하셨어요! 코드를 쉽게쉽게 잘 짜시는거 같아서 부럽고 대단하시다고 느낍니다. 배워가는 것은 많은데 언제나 리뷰를 제가 다는게 맞을지 고민을 하다 보니 리뷰 개수가 많지 않네요. 너무 고생하셨습니다. 존경하고있습니다. 멋져용

import { atom } from "recoil";
import userData from "../data/userdata.json";
import chattingdata from "../data/chattingdata.json";

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저는 어떤 부분에 대해서 recoil을 사용할지 도통 모르겠었는데, 현재 내 정보와, 친구목록,모든사용자의 정보, 채팅 데이터를 설정해 사용하게 하신 것이 대단해요. 아이디어 배워갑니다!

</Box>
);
};

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

div와 style component가 둘 다 나와서 , 통일하면 더 좋을 것 같습니다

</Container>
);
};

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

메모하는 칸이 귀여워서 input해보려고 되게 노력했는데 input은 아직 구현되지 않았군요 속았습니다잇 ㅠ

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

앗ㅠㅠㅎㅎ 어젯밤까지 붙잡고 있다가 input으로 json 데이터를 바꾸는 게 힘들다는 걸 알고... 다른 방법을 고민해보기로 접어뒀었는데 따로 표시라도 해둘 걸 그랬네요😅 의도치 않게 속여서 죄송하다는 말과 함께.. 구현되도록 힘써보겠습니다!!

}
}
};

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저도 시간관련 함수를 따로 빼서 구현하지는 않았지만, 재사용 할 가능성 때문에, 공부해서 빼서 쓰려고 해요. 빼서 구현한다면 더 나아질 것 같아요 저도 빼 봐야죠ㅎㅎㅎ...
또 메시지에서 시간은 표시되고 있지 않고 있어서 너무 아쉬워요

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저도 리뷰하면서 많이 배우는 게 생겨서, 그 점 활용해서 시간 표시해보려고 합니다. 수정할 때 말해주신 것처럼 파일을 따로 빼서 써보려구요!!

ref={inputBoxRef}
/>
{isFocused ? (
<SendButton onClick={handleSendButtonClick}>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 부분 저는 클릭으로 메시지 보내는 작동이 안돼는데, 코드로는 문제가 없다고 생각해요.저만 이런건가요? 왜 그럴까요? 여러분 도와주세요

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저도 이 문제를 해결하려고 이리저리 시도해보았지만...🥲 결국 실행되지 못한 채로 제출하게 되었네요... 다른 리뷰에 해결책을 적어주셔서!! 해결해보도록 하겠습니다!!

Copy link

@kyuhho kyuhho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

긴 시험기간동안 고생하셨습니다!!😊

Comment on lines +172 to +173
{isFocused ? (
<SendButton onClick={handleSendButtonClick}>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 부분 저도 예전에 겪었던 문제입니다! 제가 알기론 <SendButton> 을 누르는 순간 onBlur가 더 높은 우선순위를 가지기 때문에 onClick이 실행되기 전에 먼저 isFocused가 false가 되면서 버튼이 없어지는 거 같습니다!
onMouseDown은 onBlur보다 높은 우선순위를 가진다고 합니다!

참고자료

Suggested change
{isFocused ? (
<SendButton onClick={handleSendButtonClick}>
{isFocused ? (
<SendButton onMouseDown={handleSendButtonClick}>

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안 그래도 뭐가 문제인가 하며 이리저리 바꿔봤었는데... 🥹 좋은 방법 알려주셔서 정말 감사해요!! 활용해서 수정하도록 하겠습니다 :)

const [keyword, setKeyword] = useState<string>("");

const filteredUsers = userArray.filter((user) =>
user.userName.toLowerCase().includes(keyword.toLowerCase())
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저는 대문자 여부를 확인하지 않았는데 이런 예외처리도 필요하겠네요! 덕분에 새로운 예외 알고갑니다😊

const userArray = useRecoilValue(userArrayState);
const friendId = chatting.users.filter((id) => id !== 0)[0];
const friendInfoById = userArray.filter((user) => user.id === friendId)[0];
const lastChat = chatting.chatList.at(-1);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

채팅방에서의 message state와 recoil로 선언된 chatArray가 같은 state가 아니다 보니, 채팅이 추가되어도 ChatList 페이지에서는 업데이트가 안되는 이슈가 있어요! 이러한 점도 고려해서 구현하면 좋을거 같습니다 ㅎㅎ

Comment on lines +35 to +48
{isMe ? (
user.memo ? (
<MemoBubble src={memobubble} />
) : isEditing ? (
<div>
<MemoBubble src={memobubble} onClick={handleBubbleClick} />
<span className="memo">•••</span>
</div>
) : (
<MyMemoBubble src={memocreate} onClick={handleBubbleClick} />
)
) : (
<MemoBubble src={memobubble} />
)}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

개인적으론 중첩 삼항연산자보단 if else 문을 사용하는게 코드 가독성 면에선 좋다고 생각하는데, 이런 점도 고려해보시면 좋을거 같아요!

width: 55px;
`;

const MemoContent = styled.span``;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

쓰이지 않는 컴포넌트를 한번씩 정리해주는 것도 좋은거 같습니다!

Comment on lines +88 to +91
const autoAdjustTextarea = (element: HTMLTextAreaElement) => {
element.style.height = "auto";
element.style.height = element.scrollHeight + "px";
};
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

input 텍스트가 길어짐에 따라 textarea가 증가하는 디테일이 너무 좋아요👍👍

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

해당 컴포넌트가 다른 곳에서 사용되지 않고 있는데, 나중에 추가적인 구현을 위함일까요?!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

앗 네네 원래는 따로 컴포넌트처럼 빼서 쓰려고 하다 input 코드 안에서 수정을 해보려 했었어요! 추후에 리뷰하며 배운 점 참고해서 추가 구현해보려고 합니다 🥲

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants